Inventory the behaviours, pin the tests, and give caret preservation a seam (BL-6681) - #8153

Stage 0 of the CKEditor retirement. Documentation only.

BEHAVIOR-INVENTORY.md is the acceptance criteria for the whole project:
sections A-K covering the selection toolbar, formatting commands, paste/drop
filtering, paste transforms, clipboard plumbing, clean-HTML extraction,
selection survival across DOM rewrites, undo/redo, startup races, which fields
get an editor at all, and incidental focus/bubble wiring. Each row cites where
the behaviour lives today and, where the code says so, the ticket behind it.

Scoped deliberately to behaviours *at risk*: those implemented by CKEditor or
in code this project will move. BloomField.ManageField's CKEditor-independent
behaviours stay where they are and are excluded, so the rows that matter aren't
diluted.

Two kinds of row earn a marker. "capture first" rows (all of section C) are
guarantees whose failure is silent, so today's real behaviour must be recorded
before any code changes -- the config string alone doesn't tell us what happens
on a drop, or on a paste from a real web page. "must NOT survive" rows are
current behaviour we intend to remove, listed so nobody faithfully reimplements
a workaround for a problem that will no longer exist.

Four behaviours need a new implementation rather than a port, and the inventory
says why for each: the BL-12357 small-caps transform depends on CKEditor
stamping "cke/id" on its own copies; bloom-preventRemoval currently allows a
deletion and then calls execCommand("undo"), the browser stack we intend to
fence off; the colour panel is superseded by Bloom's own colour dialog; and
caret survival during reader markup should get strictly better, since offset
anchors don't perturb the DOM the way bookmarks do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…BL-6681)

Stage 0 characterization tests, inventory rows G4/G5. New file only.

EditableDivUtils.makeSelectionIn and getElementSelectionIndex predate CKEditor,
and the retirement plan builds its bookmark-free selection anchors on them
(PLAN.md 4.3). Two reasons to pin them before that work starts:

- makeSelectionIn's divBrCount parameter, which disambiguates a caret position
  around <br> elements, has no in-tree caller that passes anything but -1
  (readerToolsModel and toolbox both do). So the exact behaviour the new anchors
  will depend on is currently exercised by nothing at all.
- getElementSelectionIndex produces only a plain character offset, so the
  round-trip established here is the contract the new capture side must preserve.

The tests also record, by needing it, that both functions hard-code
parent.window.document.getElementById("page") and operate on that iframe's
window. That is why the harness has to build such an iframe. The replacement
API should instead take a document/root, which is both testable and usable
outside the page frame.

Verified these can fail, rather than trusting a first-run pass: temporarily
changing the divBrCount-1 expectation from 2 to 99 produced the expected
failure, so the assertions do observe the DOM rather than passing vacuously.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Progress log only. Documents where Stage 0 stopped and why, so the work can be
picked up without rediscovering any of it.

Two local-environment problems block the rest of Stage 0, and both have traps
worth warning about:

- node_modules in src/BloomBrowserUI is stale: react-dom 17.0.2 installed where
  package.json and the lockfile both require 18.3.1, so anything importing
  react-dom/client fails to resolve. That breaks three spec files at load time
  and blocks launching Bloom, which is what three of the four remaining Stage 0
  items need.
- pnpm can't run here: volta's default Node is 22.12.0, pinned pnpm needs 22.13+,
  and forcing it past that makes pnpm want to purge node_modules. The log warns
  explicitly against setting CI=true or confirmModulesPurge=false to get through,
  since that wipes node_modules silently. It also records the workaround that
  does work for React-free specs: invoking vitest directly rather than through
  the pnpm launcher.

Also records the already-fixed third problem for the benefit of other clones: a
stale husky v4 hook hard-coding yarn was running because core.hooksPath was
unset, fixed per .githooks/README.md rather than with --no-verify.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Progress log only.

My earlier note said to upgrade Node through volta. That was wrong. ReadMe.md's
Building section says to install vite-plus (vp) globally, which reads
.node-version and the packageManager field and provides the correct node and
pnpm, and it says volta was dropped precisely because it does not fully support
pnpm -- which is the exact failure hit here.

The machine state made this easy to misread: volta was on PATH serving Node
22.12.0, vp was not installed, and the yarn-era volta field is still frozen in
the stale output/browser/package.json build copy. So it looks like a volta
project until you read the docs. .node-version says 24.13.0 and agrees with
devEngines; nothing was reading it.

The boxed warning now records the documented sequence (install vp, then
./init.sh) and two traps: don't reach for volta, and don't force pnpm past its
purge prompt with CI=true or confirmModulesPurge=false. It also notes that
init.sh's closing "pnpm run build" is the one legitimate exception to the
"don't run the full pnpm build" rule in AGENTS.md, since it is the documented
setup path and there is no dev server running at that point.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Stage 0 prep commit. Extracts the save-selection/restore-selection bracket out
of toolbox.ts's keystroke pipeline into markupSelectionPreservation.ts, with the
same CKEditor-bookmark implementation it always had.

Why, rather than doing this later with the rest of the work: this is the one
place where the CKEditor-retirement plan's "integration points are one-line
dispatches" does not survive contact with the code. Replacing bookmarks with
character-offset anchors would otherwise mean rewriting about a hundred lines of
handleKeyboardInput -- interleaved createBookmarks, comment stripping, the
async-markup double-bookmark dance for BL-10133, cleanUpNbsps and selectBookmarks
-- inside a churn-prone file, in the middle of a months-long project. Doing the
mechanical part now, on its own, means the eventual change swaps four function
bodies and touches this pipeline not at all.

The seam is four functions: boxParticipatesInMarkup, saveSelectionForMarkup,
restoreSelectionAfterMarkup, and restoreAndResaveSelectionForMarkup (the
asynchronous path's restore-then-record-again). The saved value is deliberately
opaque -- typed unknown[] -- because the planned implementation stores a
character offset rather than DOM markers.

Behaviour is unchanged except for one deliberate difference, commented at the
site. The async path previously did createBookmarks unguarded; had the editor
reported no selection there it would have thrown, inside an async function
nobody awaits, so the pass died half-done via an unhandled rejection with
comments already stripped and marker spans possibly left in the DOM. It now
abandons the pass cleanly, which is how the first save has always behaved.

Also corrects a comment that has been wrong for some time: it claimed
ArithmeticTemplate number boxes get no editor "because the logic that invokes
WireToCKEditor is looking for classes like bloom-content1". ckeditableSelector
in utils/shared.ts explicitly includes .Equation-style, added for that template.
The real no-editor case is a box whose computed cursor is not-allowed.

Verified: full front-end suite green (591 passed), project typecheck clean, both
changed files lint clean. Live verification of caret behaviour during reader
markup (inventory rows G1-G3) is still outstanding and comes with the live-Bloom
session.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Progress log only. Stage 0's code work is complete; what remains needs a
running Bloom.

The environment is fixed. react-dom was still 17.0.2 after init.sh, because its
backgrounded pnpm install had failed under the wrong Node; a direct vp install
in src/BloomBrowserUI repaired it, and the full front-end suite now passes (591
tests). The boxed warning is rewritten around what actually matters going
forward: vp is installed and works, but Volta still wins for bare "node" and
cannot be outranked by reordering the User PATH, because Windows always puts
Machine-scope PATH first. So project commands must go through vp. It also
records why removing Volta is a trade rather than pure cleanup: four maintenance
worktrees still carry yarn-era volta fields.

Also flags the verification gap left by the prep commit: toolboxSpec covers only
cleanUpNbsps and removeCommentsFromEditableHtml, not the keystroke pipeline, so
inventory rows G1-G3 must be checked against a running Bloom before the PR. That
is now the first item in Next actions rather than an afterthought.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Progress log only. The environment is now correct and init.sh runs clean.

Three problems were layered, each masking the next, and the intermediate states
were misleading enough to be worth recording:

- Volta's shim directory was in Machine PATH, and Windows evaluates Machine
  before User, so vp's User-scope bin could never win. No User-PATH reordering
  can fix that; the Machine entry had to go.
- Removing it broke far more than node. Every binary in %LOCALAPPDATA%\Volta\bin
  is a shim whose whole body is `volta run "$(basename $0)" "$@"`, so with
  volta.exe off PATH, pnpm, pnpx, reviewable, nx, nx-cloud and
  chrome-devtools-mcp all failed with "volta: command not found". That, not
  init.sh itself, is what reported it could not find volta. Correcting my earlier
  advice: that directory is NOT harmless just because it contains no node.
- vp does not put pnpm on PATH at all (its shims are node, npm, npx, corepack,
  vpx, vpr); it runs pnpm internally for its own subcommands, but init.sh calls
  bare pnpm. Fixed with the corepack vp ships, installing a pnpm shim into
  vp's bin so it honours each package.json's packageManager field.

Volta was then restored via User PATH rather than Machine, appended so it falls
after vp's bin: node resolves to vp 24.13.0, pnpm to vp 11.5.2, volta remains
available at 2.0.2 for the four yarn-era maintenance worktrees, and the
volta-installed global tools work again. Nothing had to be uninstalled, so the
trade-off noted in the previous entry no longer applies.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Stage 0 live verification, plus the harness so it need not be rebuilt.

verifyCaretPreservation.mjs types a character mid-word in a bloom-editable and
checks where the caret lands after the markup pipeline's 500ms debounce. Run
twice independently against a running Bloom, PASS both times: text becomes
"housze", the caret sits at offset 5 immediately after the typed character, no
cke_bm_* bookmark spans are left in the DOM, and no stray zero-width spaces.
That exercises the wiring of all four functions extracted in 2707d98a8 and shows
the bookmark lifecycle still balances -- real evidence the prep commit did not
break the app's most delicate keystroke code, which has no unit coverage.

It deliberately records what is NOT covered, because the gap is easy to forget:
with no tool active updateMarkup never runs, so the DOM is unchanged between save
and restore. The case bookmarks exist for -- markup rewriting the DOM around the
caret -- plus G2's async path (BL-10133, where the prep commit made its one
deliberate behaviour change) and G3 longpress all remain open. They need a
Decodable or Leveled Reader book: a Basic Book's toolbox offers only Talking Book
and "More...", and neither became visible.

Two obstacles are recorded in PROGRESS.md because they will recur: a Team
Collection that is Disconnected blocks editing its existing books entirely, and
the way through is to create a new book rather than to switch collections (which
would mean driving a WinForms chooser CDP cannot reach); and
toolbox.toolboxIsShowing() gates markup, so the pane must genuinely be open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Docs only, following the rebase onto master.

Master's BL-16558 changed the decodable and leveled reader tools so they no
longer rewrite the DOM to show violations: they paint ::highlight()
pseudo-elements over live Range objects (textHighlightManager.ts,
readerHighlights.ts). Talking Book's current-sentence highlight works the same
way. That invalidates two of this plan's premises and adds a requirement.

- The argument against CKEditor bookmarks gets stronger rather than weaker.
  It used to be "a marker span briefly confuses the markup routine mid-word".
  It is now "inserting and removing nodes around the caret is exactly the churn
  live Ranges cannot survive", i.e. bookmarks are hostile to the architecture
  the app has just adopted.
- Our own restore paths inherit the trap. A Tier 1 undo restores an editable's
  innerHTML, which rebuilds its text nodes and collapses every Range pointing
  into them, so the highlights vanish with no error at all. Recorded as an
  explicit obligation on reinitializePageAfterRestore(): repaint via
  textHighlightManager and audioTextHighlightManager. This is the same trap
  BL-16558 had to fix by moving updateMarkup() after cleanUpNbsps and making
  cleanUpNbsps write innerHTML only when it actually converted something.

Adds inventory rows G6/G7 for the highlights, with the acceptance test that
toolbox.ts itself prescribes: type in a Leveled Reader book and watch the
over-long sentences stay highlighted. Also records that G1 is now verified on
both halves -- the automated harness, plus John checking real typing in a
decodable reader, which is the case automation could not reach.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Comment accuracy only; no behaviour change.

The comment claimed the guard replaced a case where the old code threw. That
overstated it. The old code did restore-then-re-save, both dereferencing
getSelection() unguarded, and the restore is still unguarded in
restoreSelectionAfterMarkup -- so a null selection there throws exactly where it
always did. The guard covers only the narrower case where the restore's
getSelection() succeeds and the re-save's immediately afterwards returns null.

Raised by the preflight review. Worth fixing rather than leaving: two comments in
this same pipeline have already misled this project by overstating what the code
does, and one of them is corrected in this very branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ntory

Devin's review of this branch was clean -- no bugs, no investigate flags -- but
three of its five informational observations were worth acting on.

- The committed verification harness hard-coded repoRoot as C:/github/BloomDesktop,
  so it only ran in the checkout it was written in; anyone else got a module
  resolution failure before reaching any CDP logic. It now derives the root from
  import.meta.url, which is the whole point of having committed it.
- There are two deliberate behaviour differences in the extraction, not one.
  restoreSelectionAfterMarkup re-reads the editor and no-ops if it has gone,
  where the old code sat inside `if (ckeditorOfThisBox)` and would have thrown.
  Unreachable in practice, since bloomCkEditor is assigned once per div and never
  cleared, but real -- and this branch already corrects two comments in this same
  pipeline that misled by overstating, so leaving a third would be poor form.
- The inventory pointed at PASTE-DROP-BASELINE.md as though it existed. It says
  so explicitly now, and why it is deliberately still outstanding.

Not acted on: the note that the async early-return leaves the debounce timer
handle stale (the pre-extraction early-return did the same, and a stale handle for
an expired timeout only costs a pointless clearTimeout), and the note that the new
spec relies on parent === window under jsdom (already explained in the spec's
header, along with the recommendation that the replacement API take a document
instead of hard-coding the page iframe).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Documentation only. The Current state block still said 'not pushed, no PR yet',
which is the first thing a resuming session reads.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>